Implement dynamic MCS selection based on signal strength in vWIFI driver #80
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jserv
requested changes
Jun 7, 2025
jychen0611
suggested changes
Jun 8, 2025
There was a problem hiding this comment.
It seems that you are adjusting the MCS based on the current signal strength. However, what if a user wants to set the MCS manually using a command like iw dev wlan0 set bitrates ht-mcs-2.4 <mcs_index>?
To support this, you could consider implementing the set_bitrate_mask() callback in your cfg80211_ops and storing the specified MCS in the corresponding vwifi interface structure.
Thanks for you suggestion ,I can try to included the MCS for maunally adjust ! |
dingsen-Greenhorn
force-pushed
the
dynamic_MCS
branch
from
79a5101 to
4e81b72
Compare
jychen0611
suggested changes
Jun 8, 2025
vwifi.c
Outdated
| @@ -88,6 +88,8 @@ struct vwifi_vif { | |||
| struct wireless_dev wdev; | |||
| struct net_device *ndev; | |||
| struct net_device_stats stats; | |||
| int manual_mcs; /* ADDED: Store user-specified MCS */ | |||
There was a problem hiding this comment.
Remove "ADDED:" prefix from the comment
vwifi.c
Outdated
| @@ -88,6 +88,8 @@ struct vwifi_vif { | |||
| struct wireless_dev wdev; | |||
| struct net_device *ndev; | |||
| struct net_device_stats stats; | |||
| int manual_mcs; /* ADDED: Store user-specified MCS */ | |||
| bool manual_mcs_set; /* ADDED: Flag to indicate manual MCS override */ | |||
There was a problem hiding this comment.
Remove "ADDED:" prefix from the comment
vwifi.c
Outdated
| @@ -1728,13 +1743,8 @@ static int vwifi_start_ap(struct wiphy *wiphy, | |||
|
|
|||
| /* Initialize hrtimer of beacon */ | |||
| pr_info("vwifi: init beacon_timer.\n"); | |||
| #if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 15, 0) | |||
There was a problem hiding this comment.
Don't modify unrelated code segment!
You should align with the latest version of vwifi.
First,the code handles MCS selection and modulation assignment, either manually (based on vif->manual_mcs_set and vif->manual_mcs) or automatically (based on signal strength). Second,The vWIFI driver constrains MCS indices to 7, 15, 23, and 31 to simplify rate calculation, testing, and implementation. This avoids the need for a comprehensive calculation of data-rates to handle all 32 MCS indices, varying streams, and channel widths. The chosen indices represent a range of modulations (BPSK to 64-QAM) for 4 streams, supporting both manual (iw commands) and automatic (signal-based) modes. The code’s design prioritizes simplicity for a virtual driver, ensuring functionality (as shown in your test output) while limiting complexity. And thanks for you thoughtful question,it might be the next PR to optimize it ! |
|
Nit: It might be better to squash all the “Fix coding style” commits. |
jychen0611
reviewed
Jun 10, 2025
scripts/hostapd.conf
Outdated
| @@ -2,7 +2,7 @@ interface=vw0 | |||
| driver=nl80211 | |||
| debug=1 | |||
| ctrl_interface=/var/run/hostapd | |||
| ctrl_interface_group=0 | |||
| ctrl_interface_group=root | |||
There was a problem hiding this comment.
The changes to the MCS configuration logic and the hostapd.conf control group setting serve different purposes and should be separated into distinct PRs.
The MCS logic affects runtime behavior of the vwifi driver, while the ctrl_interface_group=root change only impacts permission control for the hostapd socket.
Splitting them would improve clarity, ease future debugging, and allow better tracking of changes.
jychen0611
reviewed
Jun 10, 2025
vwifi.c
Outdated
| const char *modulation; | ||
| if (vif->manual_mcs_set) { | ||
| mcs_index = vif->manual_mcs; | ||
| switch (mcs_index) { |
There was a problem hiding this comment.
According to IEEE 802.11n, MCS indices 7, 15, 23, and 31 all use 64-QAM modulation.
You should revise the descriptions associated with each MCS index accordingly, or consider modifying your design if your intention was to differentiate based on modulation type.
ref:
mcsindex
|
|
jychen0611
suggested changes
Jun 12, 2025
vwifi.c
Outdated
| { \ | ||
| .bitrate = (_rate), \ | ||
| .hw_value = (_hw_value), \ | ||
| for file in ${SOURCES} |
There was a problem hiding this comment.
Why is there a shell script embedded in the kernel module???
Use command
$ clang-format -i *.[ch]
to maintain a consistent coding style in your implementation!
dingsen-Greenhorn
force-pushed
the
dynamic_MCS
branch
13 times, most recently
from
c556347 to
c96febb
Compare
vwifi.c
Outdated
Comment on lines
1441
to
1442
| /* Checks vif->manual_mcs_set to use vif->manual_mcs if set; | ||
| * Assigns modulation string for manual MCS ; else auto change based |
There was a problem hiding this comment.
Nit: Use imperative mood
Edit: solved
vwifi.c
Outdated
| @@ -1403,8 +1403,19 @@ static int vwifi_get_station(struct wiphy *wiphy, | |||
| sinfo->tx_failed = vif->stats.tx_dropped; | |||
| sinfo->tx_bytes = vif->stats.tx_bytes; | |||
| sinfo->rx_bytes = vif->stats.rx_bytes; | |||
|
|
|||
|
|
|||
vwifi.c
Outdated
| sinfo->tx_bytes, sinfo->rx_bytes); | ||
|
|
||
| /* Dynamic modulation based on signal strength */ | ||
| /* Checks vif->manual_mcs_set to use vif->manual_mcs if set; |
above picture demonstrate the random signal strength mapping to different MCS .(code from MCS 24-31) |
jychen0611
suggested changes
Jun 27, 2025
dingsen-Greenhorn
force-pushed
the
dynamic_MCS
branch
3 times, most recently
from
90fd779 to
049ab83
Compare
Got it, solved ! thanks. |
jychen0611
suggested changes
Jun 27, 2025
vwifi.c
Outdated
| @@ -2162,6 +2393,95 @@ static int vwifi_leave_ibss(struct wiphy *wiphy, struct net_device *ndev) | |||
| return 0; | |||
| } | |||
|
|
|||
| /* Callback to handle manual MCS and GI of related bitrate configuration via iw | |||
| */ | |||
| /* Define vwifi_set_bitrate_mask based on kernel version */ | |||
jychen0611
suggested changes
Jun 27, 2025
| return -EINVAL; | ||
| } | ||
|
|
||
| pr_info("vwifi: set_bitrate_mask called for dev %s, link_id %u, peer %pM\n", |
There was a problem hiding this comment.
Also add kernel version check for this pr_info.
dingsen-Greenhorn
force-pushed
the
dynamic_MCS
branch
4 times, most recently
from
c024fc3 to
7858f7d
Compare
jychen0611
suggested changes
Jun 27, 2025
vwifi.c
Outdated
| dev->name, link_id, peer ? peer : vif->bssid); | ||
| #else | ||
| pr_info("vwifi: set_bitrate_mask called for dev %s, peer %pM\n", dev->name, | ||
| link_id, peer ? peer : vif->bssid); |
dingsen-Greenhorn
force-pushed
the
dynamic_MCS
branch
from
7858f7d to
2fd8e42
Compare
jserv
requested changes
Jun 27, 2025
This commit enhances the vWIFI driver by implementing dynamic Modulation and Coding Scheme (MCS) selection in the `vwifi_get_station` function, adjusting the MCS index based on signal strength. After implement dynamic MCS can avoid TX power waste for a bad channel quality.
Implement the set_bitrate_mask callback in cfg80211_ops to support manual MCS settings using `iw dev <interface> set bitrates ht-mcs-2.4 <mcs_index>`, addressing reviewer feedback. Store the selected MCS in vwifi_vif->manual_mcs and track its state with vwifi_vif->manual_mcs_set. Support MCS indices 7, 15, 23, and 31, with validation and logging. Tested with `iw dev vw1 set bitrates ht-mcs-2.4 15`, achieving MCS 15 at 130.0 MBit/s (bitrate calculation pending refinement to ~52 MBit/s). Test commands format $sudo ip netns exec ns1 iw dev vw1 link $sudo ip netns exec ns1 iw dev vw1 set bitrates ht-mcs-2.4 15 /*(changable 7,15,23,31)*/ $sudo ip netns exec ns1 iw dev vw1 link
Update the vwifi_set_bitrate_mask callback to support the full range of MCS indices 24 through 31, aligning with the 4-spatial-stream configuration in nf_band_2ghz. This change enabling support for manual MCS from 24-31 coding schemes (1/2, 3/4, 2/3, 5/6) and modulations (BPSK, QPSK,16-QAM, 64-QAM) as defined by the IEEE 802.11n specification. The callback now rejects indices outside 24–31, ensuring compliance with 4-stream capabilities and allowing comprehensive rate testing (26–260 Mbps). The auto MCS selection, due to this function is based on only signal strength, while the MCS should construct with modulation and coding scheme, which the coding scheme is related with BER (bite error rate), previous vWiFI only implement random signal strength, meaning that bit error rate should also based on the channel state info.
- Adjust MCS indices 24 to 31 to match spec table's modulation (BPSK to 64-QAM) and coding rates (1/2 to 5/6). - Revise signal strength thresholds (-45 to -75 dBm) to reflect realistic SNR requirements for each modulation scheme. - Preserve random signal generation logic using rand_int_smooth. - Ensure consistency with the reference table's structure and parameters.
…CS 0-31 This commit enhances the vwifi driver and test script to fully support HT mode with MCS 0-31, dynamic guard interval (GI) selection (0.8 µs and 0.4 µs), and robust testing. Also, for dump station simplify sta_vif validation and remove ambiguous sta_vif == ap_vif check. Key changes: - Adde `gi_mode` module parameter to vwifi.c to enable dynamic switching between long GI (0.8 µs) and short GI (0.4 µs), replacing hardcoded `vif->gi = VWIFI_TXRATE_GI_800NS` in `vwifi_set_bitrate_mask`. - Update `vwifi_set_bitrate_mask` to use `gi_mode` and added logging for GI and MCS settings. - Enhance `vwifi_get_station` to report GI correctly in `sinfo->txrate` and `sinfo->rxrate` with `RATE_INFO_FLAGS_SHORT_GI`. - Update `test_vwifi_bitrates.sh` to: - Test MCS 0-31 with both long and short GI on `vw1` and `vw2`. - Validate bitrates against `ht_mcs_table` (`rate_800ns`, `rate_400ns`). - Add error handling for `gi_mode` writes and connection checks. - Include debug output for `wpa_supplicant` and `iw dev link`. - Suggested debugfs fallback for per-interface GI control if `gi_mode` fails. - Replaced the sta_vif == ap_vif check with a direct null check on sta_vif. The previous comparison between station and access point virtual interfaces (sta_vif == ap_vif) was logically unsound, as these represent distinct roles in Wi-Fi operation and should not be equated. Returning -ENONET when sta_vif equals ap_vif was unclear and potentially masked deeper issues. The new logic simplifies the flow: if no valid sta_vif is found after iterating, return -ENONET; otherwise, continue normally. Also removed an unused assignment to `ret = 0`. These changes ensure compliance with reviewer requirements for HT mode, MCS 0-31, GI selection, and dynamic MCS selection, with robust testing and validation.
…egrate test script
This commit refactors the vwifi driver to store Guard Interval (GI)
information solely in cfg80211_bitrate_mask, eliminating the redundant short_gi variable,
and introduces a spatial stream index for future optimization.
It also adds a comprehensive comment block for manual_mcs and updates the test script to
validate all MCS indices (0–31) with sgi-2.4 and lgi-2.4.
changes:
- vwifi_vif:
- Remove short_gi, storing GI in vif->bitrate_mask.control[NL80211_BAND_2GHZ].gi.
- Add spatial_streams field (int, default 1) for future multi-stream support.
- Add manual_mcs field with detailed comment block explaining its role in storing
the highest enabled MCS for consistent bitrate reporting.
- vwifi_set_bitrate_mask:
- Store GI in vif->bitrate_mask.control[NL80211_BAND_2GHZ].gi, removing short_gi usage.
- Ensures vif->manual_mcs is set to the highest
- Loop iterates through all possible MCS indices (0–31) and logs each enabled index
- vwifi_get_station:
- GI logic using vif->bitrate_mask.gi for short (0.4µs), long (0.8µs), or default GI.
- Correct pr_info format string for modulation and coding_rate alignment.
- Configure sinfo->rxrate/txrate with vif->bitrate_mask.gi and vif->manual_mcs.
Test script (test_vwifi_bitrates.sh):
-Updated to test MCS 0–31 with sgi-2.4 and lgi-2.4:
- Test header (Testing MCS <mcs> with <gi> on vw1).
- GI status (Set GI to long/short).
- iw dev vw1 link output (MAC, SSID, freq, RX/TX, signal, bitrates).
- Success/failure message with actual vs. expected bitrate.
- Add expected bitrate arrays for lgi-2.4 and sgi-2.4.
- Enhance stability with 2s retry sleep, 1s sleep after iw set bitrates, and 2s setup delay.
- Ensure cleanup resets bitrate.
Testing:
- Verified GI (0.4µs/0.8µs) and MCS (0–31) with iw dev vw1 set bitrates and iw dev vw1 link.
Resolve type mismatch error in vwifi_set_bitrate_mask for kernels < 5.19.2, where cfg80211_ops. set_bitrate_mask lacks the link_id parameter, causing a build failure. Add conditional compilation using LINUX_VERSION_CODE to define vwifi_set_bitrate_mask with or without link_id, ensuring compatibility with kernels >= 5.19.2 and older. Preserve all MCS and GI functionality. Changes: - Define vwifi_set_bitrate_mask with link_id for kernels >= 5.19.2 and without for older kernels. - Update logging to include link_id only for newer kernels. - Retain vwifi_vif fields (manual_mcs, manual_mcs_set, spatial_streams) and MCS logic (log all enabled MCSs, select highest, default to MCS 0). Fixes: Type mismatch error for set_bitrate_mask on kernels < 5.19.2
dingsen-Greenhorn
force-pushed
the
dynamic_MCS
branch
from
2fd8e42 to
3576b01
Compare
Solved ! thanks for the reminder. |
jserv
requested changes
Jun 28, 2025
Thank you for the suggestion. I understand the preference for a cleaner history, but due to the nature of the implementation, each commit represents a distinct and logically separable feature or fix. Merging them further would obscure the rationale and traceability of each functional change. Therefore, I believe keeping at least 4–5 commits is necessary for clarity and future maintenance. Alternatively, we can split the changes into separate PRs, each focusing on a specific functional unit . This would make the scope of each PR clearer and simplify review without losing commit granularity. Let me know if this direction would be preferable. |
|
Issue 1: The GI and MCS index settings are not compatible with each other. Issue 2: There appears to be a problem with executing the station dump command. |
|
Issue 3: The station dump doesn't support manual MCS. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit enhances the vWIFI driver by implementing dynamic Modulation and Coding Scheme (MCS) selection in the
vwifi_get_stationfunction, adjusting the MCS index based on signal strengthAfter implement dynamic MCS can avoid TX power waste for a bad channel quality